Skip to content

Do not emit a leading boolean operator for the first enabled filter rule (fixes #76) - #563

Open
iacsha wants to merge 1 commit into
OpenIntegrationEngine:mainfrom
iacsha:fix/76-filter-first-operator
Open

iacsha wants to merge 1 commit into
OpenIntegrationEngine:mainfrom
iacsha:fix/76-filter-first-operator

Conversation

@iacsha

@iacsha iacsha commented Nov 5, 2026

Copy link
Copy Markdown

Don't emit a leading boolean operator for the first enabled filter rule (fixes #76)

Problem

JavaScriptBuilder.appendFilterScript builds the filter as one boolean expression,
prefixing every enabled rule with its operator:

builder.append("function doFilter() { ... return (");
for (...) {
    String operator = "";
    if (rule.getOperator() == AND) operator = " && ";
    else if (rule.getOperator() == OR) operator = " || ";
    builder.append(operator + "(filterRule" + i + "() == true)");
}

The operator of the first term is emitted too. Normally the first rule's operator is
NONE/null, so nothing is prepended. But when the first step is disabled, the next
enabled rule carries the AND/OR it was given relative to the step above it, so the
expression starts with a dangling operator:

function doFilter() { ... return ( && (filterRule1() == true) || (filterRule2() == true)); }

That is invalid JavaScript, so the channel fails to deploy. Multiple users have hit this
in production.

Fix

Skip the operator for the first enabled rule — it has nothing to combine with:

boolean firstRule = true;
for (...) {
    String operator = "";
    if (!firstRule) {
        if (rule.getOperator() == AND) operator = " && ";
        else if (rule.getOperator() == OR) operator = " || ";
    }
    firstRule = false;
    builder.append(operator + "(filterRule" + i + "() == true)");
}

The channel now deploys and the disabled step does not execute, which is the expected
behavior described in the issue.

Tests

Added JavaScriptBuilderTest.testFilterDisabledFirstRuleNoLeadingOperator: a disabled
first step followed by an enabled AND rule must not produce a leading operator in
doFilter(). The test fails on main and passes with this change; the existing
JavaScriptBuilderTest cases are unchanged (7 tests, 0 failures).

A note on approach

The issue raised three possible directions: disallow saving this configuration, fix the
generated code, or reorder so the first enabled rule becomes the first. This PR takes the
second — fixing the generated code — as the least invasive change that produces the
issue's stated expected behavior. Happy to switch to validation-on-save or reordering if
you'd prefer one of those.

…ule (fixes OpenIntegrationEngine#76)

appendFilterScript prefixed every enabled rule with its AND/OR operator, including the first term. When the first step is disabled the next enabled rule carries an operator, so doFilter started with a dangling && or ||, which is invalid JavaScript and fails to deploy. Skip the operator for the first enabled rule.

Signed-off-by: iacsha <122627209+iacsha@users.noreply.github.com>
@iacsha
iacsha force-pushed the fix/76-filter-first-operator branch from 062d0bb to 99c044d Compare November 6, 2026 19:47
@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown

Test Results

  141 files  ±0    141 suites  ±0   3m 20s ⏱️ +21s
  920 tests +1    920 ✅ +1  0 💤 ±0  0 ❌ ±0 
1 052 runs  +1  1 046 ✅ +1  6 💤 ±0  0 ❌ ±0 

Results for commit 99c044d. ± Comparison against base commit 892e33f.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Having the first filter step be disabled and tied to the second step causes problems

2 participants